Skip to content

Conversation

ericnakagawa
Copy link
Contributor

Fixes 8052
Fixes 8068

  • Removed .includes() and updated corresponding CodePen
  • Added line-highlighting on first 2 examples

Copy link
Contributor

@lacker lacker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a few style nitpicks but I think this is good to go once those are dealt with.

```

If you wanted to listen to updates to the value, you could use the `onChange` event just like you can with controlled components.
If you wanted to listen to updates to the value, you could use the `onChange` event just like you can with controlled components, however you would _not_ pass the value you saved to the component.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a phrasing nitpick - can you break this up into two sentences like:

If you wanted to listen to updates to the value, you could use the onChange event just like you can with controlled components. However, you would not pass the value you saved to the component.

}
let value = event.target.value;
let checked = this.state.checked; // copy
if(!checked[value]) { checked[value] = true; } else { checked[value] = false; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you put a space after if and break this up into multiple lines so it matches the style of other parts?

handleSubmit(event) {
alert("Boxes checked are: '" + this.state.checked + "'");
alert("Boxes checked: " +
(this.state.checked.A?"A ":"") +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you put spaces around the ? and : operators when you're ternarying

handleSubmit(event) {
alert("Boxes checked are: '" + this.state.checked + "'");
alert("Boxes checked: " +
(this.state.checked.A ? "A ":"") +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spaces around the : too, plz

@gaearon
Copy link
Collaborator

gaearon commented Oct 26, 2016

Thank you!

gaearon pushed a commit that referenced this pull request Oct 27, 2016
* Reapplied fixes to updated docs from master

* Reapplied fixes to Forms, removed ES2016 function includes()

* Missing carriage return

* Adding back some line breaks

* Making requested changes.

* Making space changes
acusti pushed a commit to brandcast/react that referenced this pull request Mar 15, 2017
* Reapplied fixes to updated docs from master

* Reapplied fixes to Forms, removed ES2016 function includes()

* Missing carriage return

* Adding back some line breaks

* Making requested changes.

* Making space changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants